Skip to content

fix(mcp): reject an unsupported flag instead of registering it as the server name - #164

Merged
ralyodio merged 1 commit into
moshcoder:mainfrom
clawedassistant26:fix/mcp-reject-stray-flag
Jul 31, 2026
Merged

fix(mcp): reject an unsupported flag instead of registering it as the server name#164
ralyodio merged 1 commit into
moshcoder:mainfrom
clawedassistant26:fix/mcp-reject-stray-flag

Conversation

@clawedassistant26

Copy link
Copy Markdown
Contributor

The bug

parseMcp consumes the flags it knows (--name, -t/--transport, -e/--env, -H/--header) and pushes everything else onto positional. A flag it does not know therefore survives as a positional and becomes the server name, with its value promoted to the command.

The most likely way to hit this is copying the engine-native syntax the PRD itself documents (claude mcp add -s user …):

moshcode mcp add -s user https://mcp.sentry.dev/mcp
name target (command) args
before -s user ["https://mcp.sentry.dev/mcp"]
after (rejected)

Because the spec is spliced verbatim into each engine's native mcp add argv, the bad name is not just cosmetic — it lands in the argv as a flag:

claude   mcp add -s user -s -- user https://mcp.sentry.dev/mcp
gemini   mcp add -s user -s user https://mcp.sentry.dev/mcp
codex    mcp add -s -- user https://mcp.sentry.dev/mcp

mcpCommand runs that fan-out with no confirmation step, so the mistake propagates to every installed engine at once. There is no error and no warning; the run reports added.

mcp install --scope user https://… was also wrong, but differently: it fell through to a stdio command server needs an explicit --name, which points the user at the wrong problem entirely.

Reproduction

Real CLI, unpatched:

$ moshcode mcp add -s user https://mcp.sentry.dev/mcp
· registering -s → user across MCP engines…

Patched:

$ moshcode mcp add -s user https://mcp.sentry.dev/mcp
✗ unknown mcp flag "-s" — mcp takes --name, -t/--transport, -e/--env, and -H/--header; put a command's own flags after --
$ echo $?
1

The fix

13 lines in src/integrations.mjs. Once name and target are resolved, a token still starting with - was never consumed as a flag, so it is a typo or an engine-native flag moshcode does not take. Reject it and name the flags that are accepted.

Deliberately scoped:

  • Only the name and command positions. A command's own flags are legitimate arguments, so mcp add tools npx -y srv and mcp add tools -- npx -y srv --port 3000 are untouched. Both have control tests.
  • Nothing after --. src/mcp-catalog.mjs states that mcp add <name> -- <cmd> … "still takes anything", so the escape hatch is not second-guessed. The check skips the target entirely when it came from cmdParts.
  • The check runs before the install/name branch so mcp install --scope … now reports the real cause instead of the misleading missing-name error.

Tests

New test/mcp-stray-flag.test.mjs, 16 tests. 8 cover the bug (short flag, long flag, misspelled supported flag, stray in the command position, the misleading install error, the error's contents, nothing reaching planMcpAdd, and a stray flag in front of a catalog name). 8 are controls asserting the opposite direction, so the fix cannot buy green by over-rejecting: flags after -- pass through, flags after the target stay arguments, every supported flag parses byte-identically, the catalog shortcut still resolves, a bare URL install still derives its name, the pre-existing missing-value guards still fire first, an unknown verb still reports an unknown verb, and a stdio install with no name still asks for --name.

Fail-before: 8 fail / 8 pass unpatched, 16/16 patched.

Full suite: 615 → 631 tests, 0 failures (pass 493 → 509, skipped 122 unchanged).

Note on the existing coverage

test/mcp.test.mjs:80 already exercises flag parsing, but its fixture is ["add", "tools", "--", "npx", "-y", "srv"] — the dash tokens sit after --, so they go to cmdParts and never reach the positional list where the defect lives. It passes unchanged, before and after.

… server name

An unsupported flag before `--` fell through the parse loop into the
positional list, so `mcp add -s user https://mcp.sentry.dev/mcp` registered
a server literally named `-s` with the command `user` and demoted the real
URL to an argument. The spec is spliced verbatim into each engine's native
`mcp add` argv, so Claude received:

  claude mcp add -s user -s -- user https://mcp.sentry.dev/mcp

Reject any token that still starts with `-` once it would become the name or
the command, and name the flags mcp does take. Anything after `--` is the
user's own command line and is left alone.
@ralyodio
ralyodio merged commit 54d4e45 into moshcoder:main Jul 31, 2026
3 checks passed
@ralyodio ralyodio mentioned this pull request Aug 1, 2026
ralyodio added a commit that referenced this pull request Aug 1, 2026
install.sh resolves releases/latest, so the sixteen commits merged since
v0.13.3 have been sitting on main unreachable — including a fix for a page
that locks browsers up.

The headline is the pit. /pit rendered every ending an account held and a form
per name under each, with no bound on either: at 50 endings x 100 names that
was 3.1 MiB of HTML and 36,082 DOM elements, and it managed to jam a browser
with no script on the page at all (#167). It now draws a window and says what
it is not drawing — 173 KiB, 1,926 elements — with a filter box over the top
that takes `eggs` as a substring and `def*` as a glob, debounced against the
API (#168).

The namespace also stopped being the one part of the product a script could
not touch: /api/moshpit/* now accepts the same API key /api/me and
/api/sessions already did (#169), and /pit/dns finally documents the
TronBrowser route for machines whose DNS is not theirs to change (#165).

  moshcode: foreign keys are enforced, and the licence package.json
  claims actually ships (#154)
  cli: help aliases exit 0 (#157), invalid integration commands fail (#160),
  `--` is honoured (#159), a BOM before a shebang no longer breaks (#158)
  skills: engines with no skills primitive are reported, not dropped (#166);
  `--name` requires a value (#156)
  mcp: an unsupported flag is rejected rather than registered as the server
  name (#164)
  pit: the namespace rules are vendored again with a drift test holding them
  to the published package (#161, #162, #163)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants